fix: DashboardTimeRangeSlider #1725
Merged
+124
−104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
noahonyejese
reviewed
Sep 6, 2024
|
@noahonyejese yep, technically the dimensions should come memoized from the first hook, except when it's still fetching the data; in this case the result could be null, and using |
|
@bprusinowski, looks good. Thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1617
This PR fixes a problem of wrong positioning of marks in
DashboardTimeRangeSlider. Due to the conversion of dates to milliseconds, and interpolation of values between min and max dates, there were some non-obvious issues with marks positioning. To see one such issue, see this link from TEST and slide through the slider values – see that 2022 and 2023 and extra-close to each other, while they should have the same spacing as previous years.To fix this issue, I changed the Slider's behavior from continuous to discrete, passing the values that actually appear in the data. It also makes sure that we use combined values from merged date dimension.
How to test